Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Per instance, cross platform library sandboxing support in wasm2c #1721

Open
wants to merge 114 commits into
base: main
Choose a base branch
from
Open

Per instance, cross platform library sandboxing support in wasm2c #1721

wants to merge 114 commits into from

Conversation

shravanrn
Copy link
Collaborator

@shravanrn shravanrn commented Oct 6, 2021 •
edited
Loading

@kripken Per our discussion here is the PR with all the wasm2c changes we needed to make to allow wasm2c to be used to sandbox libraries in the Firefox browser. (Fyi @deian) Briefly here are some of the more notable changes made)

  • Windows/msvc support for runtime, codegen and builtins as well as support for a wider list of platforms/architectures/compilers including Android (arm architectures), Windows 32-bit etc.
  • Support for growable indirect function call tables
  • Support for library sandboxing apis --- wasm function type index lookup (so host can add new callbacks), make heaps aligned to 4gb for compatiblity with the RLBox sandboxing framework
  • Allow multiple sandbox instances of the same library by moving global vars into a context structure that is passed to every function
  • Migrated from emscripten to use of wasi-clang
  • Upstream libc-wasi does not support windows, so I have written a minimal cross platform wasi support which includes just basic wasi function (just enough to get simple IO libraries running. Nothing complicated like networking or filesystem is supported)
  • Removed use of per function call signal handler, setjmp/longjmp as this slows transitions
  • Removed name mangling to simplify function lookup
  • Remove stack depth counting as this adds unnecessary overhead
  • A debugging aid built directly into wasm2c generated code that is similar to valgrind like shadow memory (significantly eases debugging of wasm)
  • A wasm2c runner that can run full applications (code that has a main), when they are compiled via wasm2c to C and then to a shared library (.so/.dll)

I think this PR will likely be a starting point for discussion and we can discuss what we need to change/update so we can land this without issues

@shravanrn
remove signal handling, longjmp, stack depth counting, move func type…
6f33f73 
… structs runtime data into wasmc module, add shared library for runtime
@shravanrn
Create a sandbox struct for all the sandbox params and change referen…
27ea992 
…ces and init code to use these
@shravanrn
Fix exports - functions are visible with original symbols, separate l…
8499ad7 
…ookup for globals, tables, memories
@shravanrn
Add lookup_wasm2c_func_index api
9961123
@shravanrn
Add apis to add/remove callbacks
1ef9cdb
@shravanrn
Make sandbox used 4GB aligned heaps, add windows mmap equivalent support
485f13c
@shravanrn
Fix memory check macro
f44a45f
@shravanrn
Add wasi support to runtime (adapted from emscripten repo)
b2c2e84
@shravanrn
Misc bugfixes
f52df71
@shravanrn
Extra comment for clairification
9df1f1c
@shravanrn
Changes for easier static and dynamic linking, bugfixes, callback tab…
1fe2c83 
…le expansion
@shravanrn
Bugfixes for wasi-clang
eeec5e7
@shravanrn
Some bug fixes and simplify future use of static linking
e96c086
@shravanrn
Fixes for MSVC/Windows compatibility
def21e5
@shravanrn
Remove/replace builtins use for windows compat
c1c13f7
@shravanrn
Minor fixes for static inline
93f5e60
@shravanrn
Remove builtin expect for windows
8ea640f
@shravanrn
More windows related fixes
0cf9eed
@shravanrn
Func storage class bugfix
7aed71b
@shravanrn
Fix storage class bug
0a92330
@shravanrn
Nit
3cf430b
@shravanrn
More linkage fixes and enable windows guard page model
1629f71
@shravanrn
Fix mmap_aligned for windows
c544b30
@shravanrn
More fixes for mmap aligned
d38698a
@shravanrn
More fixes for windows os_mmap_aligned
85f0730
@shravanrn
Remove windows mmap unecessary memory reserve and make mmap more robust
830f18e
@shravanrn
Format os specific files
80b767d
@shravanrn
Cleanup macros to enable explicit bounds checks
497efba
@shravanrn
Nit - bugfix for memory init
8f00744
@shravanrn
Emit const qualifiers so compilers can optimize better
138bf0a
@kripken
Copy link
Member

kripken commented Oct 7, 2021

@sbc100

Just in terms of reviewing though it seem like there are two fairly large changes here being combined

Would it be practical to split those two, @shravanrn ? If so that sounds good to me. The work to split it out might be saved in being able to land one part first and not keep discussions on a single big PR (that waits on all items for landing).

But, if it's not practical to split out, I think it's ok for me to review as it is.

@shravanrn
Copy link
Collaborator Author

Sorry for the delay! Some last minute deadlines on my side. I'll be back to updating this PR and addressing the comments in a day or two

@kripken kripken mentioned this pull request Oct 15, 2021
Merged
@shravanrn
Copy link
Collaborator Author

@kripken @sbc100 As you suggested, i think it may be easier to upstream this in batches. So, rather than landing this PR, @talg from our sandboxing team will be submitting PRs to upstream these changes in batches. This should allow us to more easily review and land things. Thanks @talg! :)

Let me know if you have any thoughts or concerns. Will close this PR if the above plan makes sense to you

@kripken
Copy link
Member

kripken commented Oct 29, 2021

Sounds like a good plan to me!

@kripken kripken mentioned this pull request Jan 20, 2022
Closed
@yhdengh yhdengh mentioned this pull request Jan 26, 2022
Merged
@shravanrn shravanrn mentioned this pull request Feb 17, 2022
Closed
@sbc100
Copy link
Member

sbc100 commented May 13, 2024

@shravanrn is most of this stuff landed now? Are these still things that remain? i.e. should we close this now?

@shravanrn
Copy link
Collaborator Author

@sbc100 At least three of these points haven't been merged in

- A minimal cross platform wasi support which includes just basic wasi function (just enough to get simple IO libraries running. 
- A debugging aid built directly into wasm2c generated code that is similar to valgrind like shadow memory (significantly eases debugging of wasm)
- A wasm2c runner that can run full applications (code that has a main), when they are compiled via wasm2c to C and then to a shared library (.so/.dll)

But I think we can close this bug, we don't need to track these here. At some point, if I get around to re-implementing this, I'll open fresh issues/PRs

@sbc100
Copy link
Member

sbc100 commented May 15, 2024

OK, closing this for now.

Thanks for all the contributions @shravanrn!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbc100 sbc100 sbc100 left review comments

@kripken kripken kripken left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shravanrn @kripken @sbc100 @glandium